Multi Part Download: Use Chunked Stream by GarrettBeatty · Pull Request #4231 · aws/aws-sdk-net

GarrettBeatty · 2025-12-11T21:26:20Z

Description

This PR refactors the buffered multipart download data handling by replacing the single large ArrayPool buffer approach with a chunked buffer stream design. The key changes include:

New ChunkedBufferStream class: A writable/readable stream that stores data across multiple small (64KB) ArrayPool buffers, avoiding Large Object Heap (LOH) allocations
New ChunkedPartDataSource class: An IPartDataSource implementation that wraps ChunkedBufferStream for seamless integration with the PartBufferManager
Removed BufferedDataSource and StreamPartBuffer classes: These are replaced by the new chunked approach
Updated BufferedPartDataHandler: Modified to use the new ChunkedBufferStream instead of StreamPartBuffer
Updated PartBufferManager and IPartBufferManager: Simplified interface and implementation to work with the new data source pattern

Motivation and Context

The previous buffering implementation used single large ArrayPool buffers which had two issues:

LOH allocations: Buffers larger than 85KB are allocated on the Large Object Heap, causing memory fragmentation and GC pressure
2GB size limitation: Single arrays in .NET cannot exceed ~2GB (int.MaxValue bytes), limiting the maximum part size

The new chunked approach:

Keeps each buffer chunk at 64KB to stay safely below the 85KB LOH threshold
Supports streams larger than 2GB by distributing data across multiple chunks

#3806

Testing

Added comprehensive unit tests for ChunkedBufferStream (878 lines) covering:
- Write and read operations
- Mode switching (write-to-read)
- Disposal and ArrayPool buffer returns
- Edge cases and error conditions
Added comprehensive unit tests for ChunkedPartDataSource (602 lines) covering:
- Construction and validation
- Read operations
- Disposal behavior
- Integration scenarios
Updated existing BufferedPartDataHandlerTests and PartBufferManagerTests to work with the new implementation
Removed obsolete tests for BufferedDataSource and StreamPartBuffer

Performance Testing

I did notice some slowdown from the older implementation

New

Total bytes per run: 5,368,709,120

Run:1 Secs:4.541582 Gb/s:9.456985
Run:2 Secs:8.800254 Gb/s:4.880504
Run:3 Secs:2.014526 Gb/s:21.319988
Run:4 Secs:3.785906 Gb/s:11.344622
Run:5 Secs:11.958203 Gb/s:3.591649
Run:6 Secs:4.626160 Gb/s:9.284086
Run:7 Secs:2.009955 Gb/s:21.368480
Run:8 Secs:1.873424 Gb/s:22.925757
Run:9 Secs:2.329907 Gb/s:18.434070
Run:10 Secs:1.595272 Gb/s:26.923105

vs

Old

Total bytes per run: 5,368,709,120

Run:1 Secs:1.636832 Gb/s:26.239515
Run:2 Secs:1.026863 Gb/s:41.826098
Run:3 Secs:2.580466 Gb/s:16.644151
Run:4 Secs:0.925399 Gb/s:46.412059
Run:5 Secs:0.841140 Gb/s:51.061266
Run:6 Secs:0.816797 Gb/s:52.583070
Run:7 Secs:0.773071 Gb/s:55.557209
Run:8 Secs:0.821784 Gb/s:52.263931
Run:9 Secs:0.816619 Gb/s:52.594513
Run:10 Secs:0.809192 Gb/s:53.077240

50TB Testing

I also tested with the 50TB file and didnt see any issues

Screenshots (if appropriate)

Ran using visual studio performance analyzer

before change

after change

no LOH allocations

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist

My code follows the code style of this project
My change requires a change to the documentation
I have updated the documentation accordingly
I have read the README document
I have added tests to cover my changes
All new and existing tests passed

License

I confirm that this pull request can be released under the Apache 2 license

Copilot

Pull request overview

This PR refactors the S3 multipart download buffering implementation to use a chunked stream approach instead of single large ArrayPool buffers. The change addresses potential issues with very large parts (>2GB) and Large Object Heap allocations while maintaining the same functional behavior.

Key Changes:

Replaces StreamPartBuffer and BufferedDataSource with ChunkedBufferStream and ChunkedPartDataSource that use multiple small 80KB ArrayPool chunks instead of single large buffers
Removes the obsolete AddBuffer overload methods from IPartBufferManager interface, consolidating on AddDataSource
Removes unused config parameter from BufferedMultipartStream constructor

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
sdk/src/Services/S3/Custom/Transfer/Internal/ChunkedBufferStream.cs	New core stream implementation that manages data across multiple 80KB ArrayPool chunks to avoid LOH and support parts >2GB
sdk/src/Services/S3/Custom/Transfer/Internal/ChunkedPartDataSource.cs	New IPartDataSource wrapper for ChunkedBufferStream with part number tracking and disposal management
sdk/src/Services/S3/Custom/Transfer/Internal/BufferedPartDataHandler.cs	Updated to create ChunkedPartDataSource instead of StreamPartBuffer for out-of-order parts
sdk/src/Services/S3/Custom/Transfer/Internal/BufferedMultipartStream.cs	Removed unused config parameter from constructor
sdk/src/Services/S3/Custom/Transfer/Internal/PartBufferManager.cs	Removed obsolete AddBuffer overload methods
sdk/src/Services/S3/Custom/Transfer/Internal/IPartBufferManager.cs	Removed obsolete AddBuffer method declarations from interface
sdk/src/Services/S3/Custom/Transfer/Internal/StreamPartBuffer.cs	Deleted - replaced by ChunkedBufferStream
sdk/src/Services/S3/Custom/Transfer/Internal/BufferedDataSource.cs	Deleted - replaced by ChunkedPartDataSource
sdk/test/Services/S3/UnitTests/Custom/StreamPartBufferTests.cs	Deleted - tests for removed class
sdk/test/Services/S3/UnitTests/Custom/BufferedDataSourceTests.cs	Deleted - tests for removed class
sdk/test/Services/S3/UnitTests/Custom/PartBufferManagerTests.cs	Updated all test cases to use ChunkedPartDataSource instead of StreamPartBuffer
sdk/test/Services/S3/UnitTests/Custom/BufferedPartDataHandlerTests.cs	Updated mocks and assertions to expect ChunkedPartDataSource instead of StreamPartBuffer
sdk/test/Services/S3/UnitTests/Custom/BufferedMultipartStreamTests.cs	Updated constructor calls to remove config parameter

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

GarrettBeatty · 2025-12-14T19:22:52Z

        #endregion

-        #region Logger
+        private readonly Logger _logger = Logger.GetLogger(typeof(PartBufferManager));


fixing this to be consistent with the other classes

philasmar

The code looks fine but I am concerned about the performance drop. Could it be that the chunk size is too small and it's creating a lot of overhead? Could you do some testing to increase the chunk size to int.MaxValue (disregarding the LOH optimization) and see if we regain the old performance? If that's the case, maybe we need to find the sweet spot for the chunk size instead of trying to completely avoid the LOH?

update chunk sizes update logic update comments

GarrettBeatty · 2025-12-22T16:39:32Z

The code looks fine but I am concerned about the performance drop. Could it be that the chunk size is too small and it's creating a lot of overhead? Could you do some testing to increase the chunk size to int.MaxValue (disregarding the LOH optimization) and see if we regain the old performance? If that's the case, maybe we need to find the sweet spot for the chunk size instead of trying to completely avoid the LOH?

talked on devex channel internally on why its okay to use 64KB default

* Increase multipart upload default part size to 8MB (#4032) * Added PutObjectResponse to TransferUtilityUploadResponse mapping (#4045) * Add missing fields to Transfer Utility request objects (#4056) * Fix issue with HeadersCollection in ResponseMapperTests * Add Progress listeners for initiated, complete, and failed for simple upload (#4059) * Add mapping of CompletemultipartUploadResponse to TransferUtilityUploadResponse (#4060) * Add multipartupload lifecycle tracking (#4061) * fix silently failing test (#4073) * Add ContentLanguage to header collection of GetObjectResponse. (#4074) * ignore isset (#4082) * Add DownloadResponse mapping (#4075) * Add additional validation to UploadPartRequests in Transfer Utility (#4083) * Update Response mapping logic for PutObjectResponse and CompleteMultipartResponse (#4086) * create tuabortmultipartuploadrequest (#4093) * Remove AmazonWebServiceResponse as base class for transfer utility repsonse objects. (#4087) stack-info: PR: #4087, branch: GarrettBeatty/stacked/12 * Add GetObjectResponse to TransferUtilityOpenStreamResponse mapping. (#4076) stack-info: PR: #4076, branch: GarrettBeatty/stacked/9 * Fix Unit test (#4108) * Add UploadWithResponseAsync api (#4105) * Add DownloadInitiated, Failed and Completed events (#4079) * Populate TransferUtilityDownloadDirectoryResponse with total objects downloaded (#4109) * Added UploadWithResponse and UploadWithResponseAsync methods to ITransferUtility interface (#4143) * Update TransferUtilityConfig and BaseDownloadRequest to add multi part download config options (#4120) * Fix content language initialization (#4168) * Multi Part Download + OpenStreamWithResponseAsync (#4130) * DownloadWithResponse with multipartdownload (#4136) * Make AddBuffer non async (#4173) * add failure policy to download directory (#4151) * fix test (#4175) * Add progress tracking for multi part download to files (#4139) * Add DownloadDirectoryWithResponse (#4141) * Optimize part streaming (#4162) * Update initiated, complete and failed events to not fire in background (#4170) * Move MaxInMemoryParts to request object (#4163) * Add error message for multipart download when using Encryption Client (#4171) * add failure policy to upload directory (#4181) * Fix download directory concurrency issue and refactor (#4180) * Update code to acquire capacity before starting task (#4182) * use throttling for discoverpart (#4183) * refactor upload directory and fix concurrency bug (#4186) * DownloadDirectory Initiated, Failed and Completed Events (#4176) * update logging (#4188) * Create UploadDirectoryWithResponse api + progress tracking events + update docs and sync exception handling (#4187) * fix test (#4190) * fix integ test build (#4191) * fix build. (#4192) * remove InternalsVisibleTo for S3 integration tests (#4194) * Fix DownloadPartProgressEventCallback race condition (#4196) * fix http concurrency (#4218) * optimize task creation (#4219) * use max size semaphore (#4220) * fix retrying valid IO exceptions * update dev configs (#4201) * Doc updates (#4229) * refactor (#4233) * code refactor src (#4235) * queue fixes (#4228) * add case sensitivity check to download directory command * Content Language Bug Fix (#4224) * Multi Part Download: Use Chunked Stream (#4231) * cleanup (#4232) * async filestream * Revert "async filestream" This reverts commit 567d815. * make ChunkBufferSize public (#4263) * async filestream (#4264) * update docs (#4265) * update docs * fix typo * Fix doc build (#4268) * Fix doc build * Revert "Fix doc build" This reverts commit 341f50b. * fix doc build * Delete generator/.DevConfigs/f8a7b6c5-d4e3-2f1a-0b9c-8d7e6f5a4b3c.json * Update changelog for S3 UploadWithResponse methods * Improve ArrayPool chunk disposal (#4272) * dispose individual chunks * Add log --------- Co-authored-by: Afroz Mohammed <afrozam@amazon.com> Co-authored-by: Garrett Beatty <gcbeatty@amazon.com> * make filestream not set async options (#4280) * make maxinmemorypartsnullable (#4279) --------- Co-authored-by: Philippe El Asmar <53088140+philasmar@users.noreply.github.com> Co-authored-by: Phil Asmar <phil.asmar@gmail.com> Co-authored-by: Afroz Mohammed <afroz429@gmail.com> Co-authored-by: Afroz Mohammed <afrozam@amazon.com>

GarrettBeatty force-pushed the gcbeatty/chunked branch 2 times, most recently from 1602121 to 5c59042 Compare December 11, 2025 21:48

GarrettBeatty requested a review from Copilot December 11, 2025 22:41

Copilot started reviewing on behalf of GarrettBeatty December 11, 2025 22:43 View session

GarrettBeatty force-pushed the gcbeatty/chunked branch from 5c59042 to 0bf170a Compare December 11, 2025 22:49

Copilot AI reviewed Dec 11, 2025

View reviewed changes

GarrettBeatty force-pushed the gcbeatty/chunked branch from 0bf170a to 7ec7d2f Compare December 12, 2025 03:40

GarrettBeatty added 2 commits December 13, 2025 15:39

chunked stream

23d6e7a

pr comments

3ac6179

GarrettBeatty changed the title ~~chunked stream~~ Multi Part Download: Use Chunked Stream Dec 13, 2025

GarrettBeatty force-pushed the gcbeatty/chunked branch from 7ec7d2f to 3ac6179 Compare December 13, 2025 20:48

GarrettBeatty requested a review from Copilot December 13, 2025 20:50

Copilot started reviewing on behalf of GarrettBeatty December 13, 2025 20:51 View session

Copilot AI reviewed Dec 13, 2025

View reviewed changes

Comment thread sdk/test/Services/S3/UnitTests/Custom/ChunkedPartDataSourceTests.cs Outdated

GarrettBeatty added 4 commits December 13, 2025 16:09

pr comments

f16e912

preallocate

c34bd70

fix logger

cd8c9be

fix loh

4a9aba1

GarrettBeatty commented Dec 14, 2025

View reviewed changes

GarrettBeatty marked this pull request as ready for review December 15, 2025 14:13

GarrettBeatty requested review from normj and philasmar December 15, 2025 14:13

philasmar requested changes Dec 15, 2025

View reviewed changes

Comment thread sdk/src/Services/S3/Custom/Transfer/Internal/BufferedPartDataHandler.cs

Comment thread sdk/src/Services/S3/Custom/Transfer/Internal/ChunkedBufferStream.cs Outdated

update comments and switchtoreadmode

866d2cb

GarrettBeatty requested a review from philasmar December 15, 2025 17:33

philasmar reviewed Dec 18, 2025

View reviewed changes

GarrettBeatty added 2 commits December 22, 2025 11:12

chunked size override

491174e

update chunk sizes update logic update comments

update unit tests

e75a473

remove comment

36798b4

GarrettBeatty requested a review from philasmar December 22, 2025 16:41

change to internal

60cd9d9

GarrettBeatty requested a review from peterrsongg December 22, 2025 17:51

philasmar approved these changes Dec 23, 2025

View reviewed changes

peterrsongg approved these changes Dec 23, 2025

View reviewed changes

GarrettBeatty merged commit cdc7874 into feature/transfermanager Dec 23, 2025
1 check passed

GarrettBeatty deleted the gcbeatty/chunked branch December 23, 2025 17:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multi Part Download: Use Chunked Stream#4231

Multi Part Download: Use Chunked Stream#4231
GarrettBeatty merged 11 commits intofeature/transfermanagerfrom
gcbeatty/chunked

GarrettBeatty commented Dec 11, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

GarrettBeatty Dec 14, 2025

Uh oh!

Uh oh!

Uh oh!

philasmar left a comment

Uh oh!

GarrettBeatty commented Dec 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

GarrettBeatty commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Testing

Performance Testing

50TB Testing

Screenshots (if appropriate)

Types of changes

Checklist

License

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

GarrettBeatty Dec 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

philasmar left a comment

Choose a reason for hiding this comment

Uh oh!

GarrettBeatty commented Dec 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

GarrettBeatty commented Dec 11, 2025 •

edited

Loading